[SPIR-V] Honor inline spir-v attributes on functions - #8616
Conversation
The [[vk::ext_decorate]], [[vk::ext_capability]], and [[vk::ext_extension]] attributes were only applied to variables, parameters, stage variables, entry-point functions, and functions carrying [[vk::ext_instruction]]. A plain function that carried them was skipped, so the attributes were silently dropped: no OpDecorate was emitted for its OpFunction, and no OpCapability / OpExtension was added to the module. Apply them when the function is registered (getOrRegisterFn), next to the existing linkage decoration. This reuses the same helpers as the variable and parameter paths: - the decoration goes through a new SpirvFunction overload of decorateWithIntrinsicAttrs, and - the capability/extension loops are factored into registerCapabilitiesAndExtensionsForDecl, now shared with the variable path. Only the literal form of ext_decorate is supported on functions; the id and string variants have no SpirvFunction-target decoration and are now diagnosed rather than silently dropped.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
| srcLoc, targetFunc, static_cast<spv::Decoration>(decorate), literals); | ||
| assert(decor != nullptr); | ||
| mod->addDecoration(decor); | ||
| } |
There was a problem hiding this comment.
addDecoration used to be used only by decorateLinkage, which always encodes the function name into params. But now two functions both carrying the same decoration will make us drop the second one. This is failing for me:
// RUN: %dxc -T cs_6_0 -E main -fcgl %s -spirv | FileCheck %s
[[vk::ext_decorate(0)]]
[noinline] uint Foo(uint x) { return x; }
[[vk::ext_decorate(0)]]
[noinline] uint Bar(uint x) { return x + 1; }
RWStructuredBuffer<uint> buf;
[numthreads(1, 1, 1)]
void main(uint3 tid : SV_DispatchThreadID) {
buf[0] = Foo(tid.x) + Bar(tid.x);
}
// CHECK-DAG: OpDecorate %Foo RelaxedPrecision
// CHECK-DAG: OpDecorate %Bar RelaxedPrecision
The second CHECK-DAG can't find the RelaxedPrecision decoration. I think adding getTargetFunc() in the hash would fix this.
There was a problem hiding this comment.
Yep. Fixed, and added a second function in the test to demonstrate.
|
|
||
| [[vk::ext_decorate_string(/* UserTypeGOOGLE */ 5636, "myType")]] | ||
| [noinline] uint Identity(uint x) { return x; } | ||
|
|
There was a problem hiding this comment.
Could you add a second function that exercises ext_decorate_id?
There was a problem hiding this comment.
Yes. I called it DecorateId. An apt name, but maybe on the nose.
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Adds support for inline SPIR-V decorations, capabilities, and extensions on ordinary functions.
Changes:
- Applies function-target decorations during registration.
- Shares capability/extension registration across variables and functions.
- Adds positive and diagnostic tests for function attributes.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
spv.intrinsicDecorate.function.hlsl |
Tests supported function attributes. |
spv.intrinsicDecorate.function.error.hlsl |
Tests unsupported decoration variants. |
SpirvInstruction.cpp |
Includes function targets in decoration equality. |
SpirvEmitter.cpp |
Reuses declaration requirement registration. |
SpirvBuilder.cpp |
Adds function decoration construction. |
DeclResultIdMapper.h |
Declares new shared helpers. |
DeclResultIdMapper.cpp |
Applies attributes during function registration. |
SpirvModule.h |
Includes function targets in decoration hashing. |
SpirvBuilder.h |
Exposes function decoration overload. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // [[vk::ext_decorate(d, ...)]] -> OpDecorate targeting the OpFunction | ||
| // [[vk::ext_capability(c)]] -> OpCapability for the module | ||
| // [[vk::ext_extension("...")]] -> OpExtension for the module | ||
| decorateWithIntrinsicAttrs(fn, spirvFunction); |
|
|
@pow2clk / @mmoult - it looks like this was merged before addressing / responding to all the feedback?
Can you follow up on these please? |
|
Yes, I agree. The fix is underway. It isn't too big, so I expect to have it up within an hour or two. |
The [[vk::ext_decorate]], [[vk::ext_capability]], and [[vk::ext_extension]] attributes were only applied to variables, parameters, stage variables, entry-point functions, and functions carrying [[vk::ext_instruction]]. A plain function that carried them was skipped, so the attributes were silently dropped: no OpDecorate was emitted for its OpFunction, and no OpCapability / OpExtension was added to the module.
Apply them when the function is registered (getOrRegisterFn), next to the existing linkage decoration. This reuses the same helpers as the variable and parameter paths:
Only the literal form of ext_decorate is supported on functions; the id and string variants have no SpirvFunction-target decoration and are now diagnosed rather than silently dropped.